Fix warnings in CLang. #15270
Fix warnings in CLang. #15270
Conversation
|
@mxnet-label-bot add [pr-awaiting-review,build] |
|
@larroy could you rebase this PR? |
|
@roywei sure. |
larroy
force-pushed
the
fix_warnings_take2
branch
from
c571f37
to
29b6d80
Compare
|
Done, could we get those small PRs that fix warnings merged? |
|
Please enable warnings as error for the ones you have fixed. |
|
@marcoabreu I think more warnings got introduced in master, if we make it so tedious to push small fixes, nobody is going to fix warnings. Warnings have to be fixed before enabling all warnings as errors, not before, otherwise some builds will fail. I will enable that once all the PRs fixing warnings are merged, it's impossible otherwise. |
In file included from ../src/kvstore/kvstore.cc:28:
../src/kvstore/./kvstore_local.h:281:23: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
auto validator = [this](const int key, const NDArray& nd, bool ignore_sparse) -> bool {
^
../src/kvstore/./kvstore_local.h:326:23: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
auto validator = [this](const int key, const RSPVal& val_rowid, bool ignore_sparse) -> bool {
^
In file included from ../src/c_api/c_api_profile.cc:35:
../src/c_api/../profiler/./profiler.h:1160:8: warning: 'mxnet::profiler::ProfileOperator::start' hides overloaded virtual function [-Woverloaded-virtual]
void start(mxnet::Context::DeviceType dev_type, uint32_t dev_id) {
^
../src/c_api/../profiler/./profiler.h:870:8: note: hidden overloaded virtual function 'mxnet::profiler::ProfileEvent::start' declared here: different number of parameters (0 vs 2)
void start() override {
^
../src/c_api/../profiler/./profiler.h:1212:8: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
[this](OprExecStat *stat) {}, name_.c_str(), dev_type_, dev_id_,
^
See also apache#14940
larroy
force-pushed
the
fix_warnings_take2
branch
from
29b6d80
to
0428fa2
Compare
|
Ok |
|
Btw I'm not talking about enabling all warnings as errors but to enable the ones you're fixing step by step. Otherwise you are going to chase a tail you'll never catch |
|
Now CI is failing, I don't think is effective to enable warnings one by one, since there are many compilers, it gets extremely tedious, if you want to do that please go ahead. |
larroy
force-pushed
the
fix_warnings_take2
branch
from
0428fa2
to
4139a50
Compare
|
As far as I remember, we only have two clang builds. Could you elaborate? I mean as far as I understand you, you looked at the clang output and fixed a specific category, right? Wouldn't that mean that this specific category is green and can be turned into an error? Sorry if I'm misunderstanding something here. That's how we did it back then, right @KellenSunderland? |
|
I understood but the problem is that there's several compilers that spew different warnings and are compiled with the same flags, I think the best would be to enable all as errors once there's no warnings in any compiler. I think adding flags one by one is too much work. |
|
I'm just talking about enabling them in our CI, not globally. Our CI is a pretty controlled environment. Would be a pity if you fix these things and others introduce them. |
|
Could you approve? this would help move this effort forward. I will look into enable warnigns as errors as soon as CI passes smoothly in the following PRs. |
|
appreciate, thanks |
In file included from ../src/kvstore/kvstore.cc:28:
../src/kvstore/./kvstore_local.h:281:23: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
auto validator = [this](const int key, const NDArray& nd, bool ignore_sparse) -> bool {
^
../src/kvstore/./kvstore_local.h:326:23: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
auto validator = [this](const int key, const RSPVal& val_rowid, bool ignore_sparse) -> bool {
^
In file included from ../src/c_api/c_api_profile.cc:35:
../src/c_api/../profiler/./profiler.h:1160:8: warning: 'mxnet::profiler::ProfileOperator::start' hides overloaded virtual function [-Woverloaded-virtual]
void start(mxnet::Context::DeviceType dev_type, uint32_t dev_id) {
^
../src/c_api/../profiler/./profiler.h:870:8: note: hidden overloaded virtual function 'mxnet::profiler::ProfileEvent::start' declared here: different number of parameters (0 vs 2)
void start() override {
^
../src/c_api/../profiler/./profiler.h:1212:8: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
[this](OprExecStat *stat) {}, name_.c_str(), dev_type_, dev_id_,
^
See also apache#14940
Description
In file included from ../src/kvstore/kvstore.cc:28:
../src/kvstore/./kvstore_local.h:281:23: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
auto validator = [this](const int key, const NDArray& nd, bool ignore_sparse) -> bool {
^
../src/kvstore/./kvstore_local.h:326:23: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
auto validator = [this](const int key, const RSPVal& val_rowid, bool ignore_sparse) -> bool {
^
In file included from ../src/c_api/c_api_profile.cc:35:
../src/c_api/../profiler/./profiler.h:1160:8: warning: 'mxnet::profiler::ProfileOperator::start' hides overloaded virtual function [-Woverloaded-virtual]
void start(mxnet::Context::DeviceType dev_type, uint32_t dev_id) {
^
../src/c_api/../profiler/./profiler.h:870:8: note: hidden overloaded virtual function 'mxnet::profiler::ProfileEvent::start' declared here: different number of parameters (0 vs 2)
void start() override {
^
../src/c_api/../profiler/./profiler.h:1212:8: warning: lambda capture 'this' is not used [-Wunused-lambda-capture]
[this](OprExecStat *stat) {}, name_.c_str(), dev_type_, dev_id_,
^
See also #14940
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.